Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu: native: don't handle pending signals in isr_thread_yield #6135

Closed
wants to merge 1 commit into from

Conversation

kaspar030
Copy link
Contributor

I've been hitting a bug were a thread wouldn't get scheduled anymore even after putting it on the respective runqueue.

I think the problem is a possibly nested call of isr_thread_yield():

  1. ISR (e.g., incoming network packet) triggers
  2. ISR sets a thread's status, calls thread_yield_higher() to trigger context switch
  3. thread_yield_higher() calls isr_thread_yield()
  4. isr_thread_yield() previously handled pending signals using native_irq_handler()
  5. native_irq_handler() calls cpu_switch_context_exit()

... somehow now the thread woken up by 2.) doesn't get scheduled anymore.

I don't know why (tm), but removing the pending signal handling from isr_thread_yield() solves this.

@kaspar030 kaspar030 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Nov 16, 2016
@OlegHahm
Copy link
Member

OlegHahm commented Nov 17, 2016

I think I found something similar when reviewing #2071.

@OlegHahm
Copy link
Member

#2071 (diff)

@OlegHahm
Copy link
Member

(And in my working branch for the 1000-native-nodes experiments, I have the same diff: OlegHahm@9930e17)

@kaspar030
Copy link
Contributor Author

@LudwigKnuepfer could you take a look?

Aren't IRQ's (signals) queued if not handled here?

If not, we need to get rid of the cpu_switch_context_exit() call. One way would be to have a version if native_irq_handler() which does not call it.
And we should probably make sure that isr_thread_yield() doesn't nest.

@LudwigKnuepfer
Copy link
Member

I can't estimate when I have time to investigate this properly. I would advise to try and figure it out on your own unless it can wait for at least two weeks.

@kaspar030
Copy link
Contributor Author

I cannot reproduce anymore. Reading through the description, this should be fixed by #6660.

@kaspar030 kaspar030 closed this Mar 21, 2017
@kaspar030 kaspar030 deleted the native_isr_race branch March 21, 2017 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants